Skip to content

Remove pre-commit from nox for lack of a config file#295

Closed
kurtmckee wants to merge 1 commit intoAdvancedPhotonSource:mainfrom
kurtmckee:rm-pre-commit-in-nox
Closed

Remove pre-commit from nox for lack of a config file#295
kurtmckee wants to merge 1 commit intoAdvancedPhotonSource:mainfrom
kurtmckee:rm-pre-commit-in-nox

Conversation

@kurtmckee
Copy link
Contributor

@kurtmckee kurtmckee commented Jan 23, 2026

When running the test suite, pre-commit simply exits with an error because there is no .pre-commit-config.yaml file.

For that reason, this PR removes the nox "lint" environment as well as references to pre-commit.

Note

In general, I recommend modifying the project's CI design so that it runs nox just as it is run locally. This will help ensure that local testing matches CI testing.

@Kvieta1990
Copy link
Collaborator

@kurtmckee I was tryingto run nox locally. with the original code (i.e., without your fix), I did see the error Command pre-commit run --all-files --show-diff-on-failure failed with exit code 1. However, it seems that pylint and test also failed for me. Can I ask how you did nox locally? what setup do we need for it? Thanks!

@kurtmckee
Copy link
Contributor Author

I'm not able to successfully run the pylint and test environments, either. I first encountered build failures, which required me to install the gfortran compiler.

https://gsas-ii.readthedocs.io/en/latest/packages.html#compilation-requirements

However, the pylint environment complains that there is no gsas_ii module, and the test environment tries to install the package's [test] extra, which doesn't exist in pyproject.toml, so as far as I can tell noxfile.py is no longer functional at all.

On closer inspection, it looks like GSAS-II has migrated to using pixi to manage its test suite, but I can't find any documentation in the project for how to use pixi. Perhaps this PR should instead simply delete the nox config file. I'll wait to hear from the devs about it.

@kurtmckee
Copy link
Contributor Author

Interestingly, I see that instructions were written for Copilot to install and gauge its contributions using nox, so the project's AI usage may be misconfigured as well.

@Kvieta1990
Copy link
Collaborator

@briantoby since we do migrate to use pixi from my understanding, should we consider remove the nox stuff, i.e., the noxfile.py since for the testing we are not using it anyhow?

@briantoby
Copy link
Collaborator

I am pretty sure the noxfile was acquired as part of a reorganization and perhaps as part of a template. I have myself never used nox nor for that matter pylint. (The code probably reflects that, but I think it is not too horrible, considering it comes from two scientists who started programming with computer cards.) We do use pixi for running self tests and offer that as an alternate installation mechanism for those who would prefer to avoid conda. (see https://advancedphotonsource.github.io/GSAS-II-tutorials/install.html#installing-gsas-ii-via-pixi and https://advancedphotonsource.github.io/GSAS-II-tutorials/install_pixi.html for more.)

As for co-pilot, that was setup quickly with the help of my supervisor, who gave a pretty minimal prompt when he was showing me how I could use that tool to make changes to the GUI. It failed, but I do use co-pilot sometimes these days anyway, but not too much for the GSAS-II code base. I suspect that it added nox to the configuration since the file was there. I see lots of stuff in that file that I would think about updating.

I'd propose removing the noxfile.py from the tree. As for the co-pilot instructions, I'm not sure when much of that gets used.

@kurtmckee I'm not sure what you are trying to test, but once GSAS-II is installed properly, the self-tests can be run individually or via pytest. The installation instructions outline a bunch of ways that the code can be installed, but the key step is getting the binaries where the code can find them. Compiling is recommended for Linux, but is more than most Windows and MacOS users want to do. If you want help/advice let me know.

@kurtmckee
Copy link
Contributor Author

@briantoby My goal was simply to run the test suite locally to confirm that the project infrastructure was healthy. I wasn't cloning the repo for installation purposes. It's common to use tox or nox for that in Python projects, so when I saw noxfile.py I just ran nox instinctively.

I'd propose removing the noxfile.py from the tree.

I'll close this PR in favor of a new PR to do just that. Thanks for reviewing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants